Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Add ArrayType rule to enforce 'T[]' or 'Array<T>' #1498

Merged
merged 1 commit into from
Aug 24, 2016

Conversation

ScottSWu
Copy link
Contributor

  • Add ArrayType / ArrayTypeNode to SyntaxWalker

This rule doesn't use type checking to detect array aliases because the alias was likely defined purposefully.

cc: @alexeagle

* Add ArrayType to SyntaxWalker
@danvk
Copy link
Contributor

danvk commented Aug 19, 2016

Not necessarily something that needs to be implemented in this PR, but our style guide is a little more nuanced than either of these options. For simple (non-generic) types we use T[]. If the type becomes more complex, we switch to Array<> for clarity, e.g. Array<{[key: string]: T}>.

type: "string",
enum: [OPTION_ARRAY, OPTION_GENERIC],
},
optionExamples: ["[true, array]", "[true, generic]"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use the OPTION_ variables here as well (no need to make a commit just to change this though)

@jkillian
Copy link
Contributor

Looks good to me @ScottSWu, if it looks good to @alexeagle as well I'm happy to merge this

@alexeagle
Copy link
Contributor

Looks great to me.

Re. the CLA:
Scott's internship ended Friday :(
But all these commits were authored as a google employee, and we've just signed your corporate CLA, so maybe that resolves it.

@ScottSWu
Copy link
Contributor Author

Oops, I've already removed the relevant email from my account (if that matters).

@adidahiya
Copy link
Contributor

Confirmed receipt of the corporate CLA from our end 👍

@adidahiya adidahiya merged commit a2beb27 into palantir:master Aug 24, 2016
soniro pushed a commit to soniro/tslint that referenced this pull request Aug 31, 2016
@nchen63 nchen63 added this to the TSLint v4.0 milestone Nov 8, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants